App migrations - #41
Conversation
📝 WalkthroughWalkthroughApplication event loops were migrated from direct Merge Risk: 🟡 Moderate · up to The PR can access invalid state during EspNowBridge shutdown, retains a concrete lint failure, and includes an error-dialog path that may block indefinitely. Merge should wait for these bounded correctness and availability issues to be fixed or explicitly accepted. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b60b41f9-1596-4f55-9be2-79b7b0bf4836
📒 Files selected for processing (37)
Apps/Brainfuck/main/Source/main.cppApps/Brainfuck/manifest.propertiesApps/Breakout/main/Source/main.cppApps/Breakout/manifest.propertiesApps/Calculator/main/Source/main.cppApps/Calculator/manifest.propertiesApps/Diceware/main/Source/main.cppApps/Diceware/manifest.propertiesApps/EpubReader/main/Source/main.cppApps/EpubReader/manifest.propertiesApps/EspNowBridge/main/Source/main.cppApps/EspNowBridge/manifest.propertiesApps/GPIO/main/Source/main.cppApps/GPIO/manifest.propertiesApps/GraphicsDemo/main/Source/Main.cppApps/GraphicsDemo/manifest.propertiesApps/HelloWorld/main/Source/main.cApps/HelloWorld/manifest.propertiesApps/M5UnitTest/main/Source/main.cppApps/M5UnitTest/manifest.propertiesApps/Magic8Ball/main/Source/main.cppApps/Magic8Ball/manifest.propertiesApps/MediaKeys/main/Source/main.cppApps/MediaKeys/manifest.propertiesApps/MystifyDemo/main/Source/Main.cppApps/MystifyDemo/manifest.propertiesApps/SerialConsole/main/Source/main.cppApps/SerialConsole/manifest.propertiesApps/Snake/main/Source/main.cppApps/Snake/manifest.propertiesApps/TamaTac/main/Source/main.cppApps/TamaTac/manifest.propertiesApps/TodoList/main/Source/main.cppApps/TodoList/manifest.propertiesApps/TwoEleven/main/Source/main.cppApps/TwoEleven/manifest.propertiesTools/bump-versions.py
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Apps/EspNowBridge/main/Source/main.cpp (1)
72-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDestroy
event_groupafter Wi-Fi unsubscription.Line 72 destroys
event_groupbefore Line 73 callsespNowBridgeTeardown(). That teardown callswifi_event_unsubscribe()for a subscription owned by this group. This violates the declared lifetime contract and can access a destroyed event group when the application closes.Move
task_event_group_destruct(&event_group)afterespNowBridgeTeardown(ctx.get()).Proposed fix
window_manager_remove(window); check(app_event_unsubscribe(&sub) == ERROR_NONE); - task_event_group_destruct(&event_group); espNowBridgeTeardown(ctx.get()); + task_event_group_destruct(&event_group);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 380c672d-28b1-4d05-b5b2-17a3d13cbd8b
📒 Files selected for processing (6)
Apps/EspNowBridge/main/Source/EspNowBridge.cppApps/EspNowBridge/main/Source/EspNowBridge.hApps/EspNowBridge/main/Source/main.cppApps/GraphicsDemo/main/Source/Main.cppApps/MystifyDemo/main/Source/Main.cppTools/bump-versions.py
🚧 Files skipped from review as they are similar to previous changes (1)
- Tools/bump-versions.py
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
Summary by CodeRabbit
Improvements
App Updates
Tools